ci: add zizmor for GitHub Actions security analysis#2648
Conversation
Run zizmor on every push to main and on PRs. Address all findings: - Add persist-credentials: false to all actions/checkout invocations - Add top-level permissions: contents: read to publish-pypi.yml - Convert github.event.release.* template expansions in comment-on-release.yml to env-var indirection in github-script to avoid code injection - Suppress the secrets-outside-env warning on the Claude Code action with an inline zizmor: ignore; moving to a dedicated GitHub environment is a separate infrastructure change
|
You are seeing this message because GitHub Code Scanning has recently been set up for this repository, or this pull request contains the workflow file for the Code Scanning tool. What Enabling Code Scanning Means:
For more information about GitHub Code Scanning, check out the documentation. |
| uses: anthropics/claude-code-action@2f8ba26a219c06cfb0f468eef8d97055fa814f97 # v1.0.53 | ||
| with: | ||
| anthropic_api_key: ${{ secrets.ANTHROPIC_API_KEY }} | ||
| anthropic_api_key: ${{ secrets.ANTHROPIC_API_KEY }} # zizmor: ignore[secrets-outside-env] |
There was a problem hiding this comment.
I guess the point is to create an environment and add the ANTHROPIC_API_KEY in there?
There was a problem hiding this comment.
Yes - that's the proper fix, but it needs a repo-settings change (create a GitHub environment with ANTHROPIC_API_KEY) which is out of scope for this code-only PR. Suppressed inline for now; happy to follow up once the environment exists.
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
The newer zizmor (1.25.x, shipped by zizmor-action v0.5.6) adds a cache-poisoning audit that flags astral-sh/setup-uv with enable-cache: true in a release: published-triggered job. Disable the cache for the release-build job - release builds are infrequent and a cold cache is fine - so the new security workflow lands clean.
There was a problem hiding this comment.
Looks good — the earlier cache-poisoning finding has been addressed (enable-cache: false + zizmor-action v0.5.6); the one remaining inline comment is a cosmetic nit about a no-op suppression.
Extended reasoning...
Overview
This PR adds a zizmor security-analysis workflow (.github/workflows/zizmor.yml, pinned to zizmorcore/zizmor-action@v0.5.6 with permissions: {} at top level and only security-events: write on the job) and hardens the existing workflows so the new check is green: persist-credentials: false on every actions/checkout, top-level permissions: contents: read on publish-pypi.yml, enable-cache: false on the release build, and the comment-on-release.yml template-injection fix that moves github.event.release.* / steps.*.outputs.result expansions into env: vars consumed via process.env (with JSON.parse for the structured outputs).
Security risks
All changes are net hardening: tighter token scopes, no persisted git credentials, no cache reuse in the publish path, and JS string interpolation replaced by env-var consumption. The JSON.parse(process.env.PREVIOUS_TAG_JSON) / JSON.parse(process.env.PR_NUMBERS_JSON) rewrites are correct because actions/github-script JSON-serializes its return value into outputs.result, so the parsed values match what the inline-template form previously evaluated to. No new permissions are granted anywhere.
Level of scrutiny
CI/CD config touching the PyPI publish pipeline normally warrants care, but every edit here is restrictive rather than permissive, so the blast radius of a mistake is a broken (not over-privileged) workflow. The zizmor.yml workflow itself is a near-verbatim copy of upstream uvicorn's reference setup.
Other factors
My earlier inline comment about the high-severity cache-poisoning finding in publish-pypi.yml has been addressed in the latest commit (enable-cache: false, action bumped to v0.5.6). The remaining bug-hunter finding is a cosmetic nit — the # zizmor: ignore[secrets-outside-env] annotation in claude.yml is a no-op under the default regular persona — which doesn't affect correctness or security and can be cleaned up in a follow-up. Maintainers (Kludex, maxisbey) have been actively reviewing and steering the PR.
| uses: anthropics/claude-code-action@2f8ba26a219c06cfb0f468eef8d97055fa814f97 # v1.0.53 | ||
| with: | ||
| anthropic_api_key: ${{ secrets.ANTHROPIC_API_KEY }} | ||
| anthropic_api_key: ${{ secrets.ANTHROPIC_API_KEY }} # zizmor: ignore[secrets-outside-env] |
There was a problem hiding this comment.
🟡 The inline # zizmor: ignore[secrets-outside-env] is a no-op for the CI job this PR adds: secrets-outside-env only fires under zizmor's auditor persona, but zizmor.yml doesn't set a persona: input so zizmor-action runs the default regular persona where this audit never triggers. This means the PR description's claim that the suppression silences "the one remaining warning" is incorrect, and the discussed follow-up (creating a GitHub environment for ANTHROPIC_API_KEY) is not actually required for a clean run with the configured persona — consider either dropping the suppression or explicitly setting persona: auditor if you want that audit enforced.
Extended reasoning...
What's happening
The PR adds # zizmor: ignore[secrets-outside-env] to .github/workflows/claude.yml:39 and the PR description says this is needed to silence "the one remaining warning" so the new zizmor job is green from the start. The in-thread discussion likewise treats creating a GitHub environment for ANTHROPIC_API_KEY as a tracked follow-up that would let this suppression be removed.
But the secrets-outside-env audit is gated behind zizmor's auditor persona. The new zizmor.yml workflow uses zizmorcore/zizmor-action@v0.5.6 with no persona: input, which means the action runs with its default — the regular persona, the same as a bare uvx zizmor invocation. Under regular (and even under the stricter pedantic), secrets-outside-env simply never fires.
Step-by-step proof (zizmor 1.25.2, the version pinned by zizmor-action v0.5.6)
- Check out this branch.
uvx zizmor .github/workflows/(regular persona, with the inline ignore) →No findings to report. Good job! (29 suppressed)- Remove the
# zizmor: ignore[secrets-outside-env]comment fromclaude.yml:39and re-runuvx zizmor .github/workflows/→ identical:No findings to report. Good job! (29 suppressed). The suppression had no effect. uvx zizmor --persona pedantic .github/workflows/claude.yml(no ignore) → still nosecrets-outside-envfinding.- Only
uvx zizmor --persona auditor .github/workflows/claude.ymlsurfaceswarning[secrets-outside-env]at line 39.
Why this matters
The suppression is dead annotation under the workflow's actual configuration. There's no functional impact on CI — the job stays green either way — but it does carry a misleading premise into the codebase and the PR record:
- The PR description says the suppression is what makes the run clean. It isn't; the run is already clean under
regular. - The in-thread agreement ("Suppressed inline for now; happy to follow up once the environment exists") tracks a follow-up — creating a GitHub environment so the suppression can be removed — that won't actually change zizmor's output. Whoever picks that up will do the repo-settings work, delete the comment, and observe nothing.
On the counter-argument that this is harmless future-proofing
It's true the comment becomes load-bearing if the team later switches to persona: auditor. But that's not what this PR sets up, and the PR text explicitly frames the suppression as required for the current configuration. A comment that documents an audit zizmor isn't running, framed as if it were, is exactly the kind of thing that costs the next reader time. If the team wants the auditor audit suite, set persona: auditor in zizmor.yml (in which case the suppression is correct and load-bearing). If not, drop the suppression to keep the annotations honest.
Suggested fix
Either:
# .github/workflows/claude.yml — remove the no-op annotation
anthropic_api_key: ${{ secrets.ANTHROPIC_API_KEY }}or, if you actually want secrets-outside-env (and the rest of the auditor suite) enforced:
# .github/workflows/zizmor.yml
- name: Run zizmor 🌈
uses: zizmorcore/zizmor-action@5f14fd08f7cf1cb1609c1e344975f152c7ee938d # v0.5.6
with:
persona: auditor(Note: enabling auditor will surface several other findings beyond this one, so it's not a drop-in change.)
|
Two pre-existing findings the new workflow will surface as
Dependabot has no delay between a release publishing and a bump PR opening, so a compromised release in a github-action could get proposed before it's noticed and yanked. A cooldown only considers versions that have been live for a window: updates:
- package-ecosystem: "github-actions"
directory: "/"
schedule:
interval: monthly
cooldown:
default-days: 14
groups:
github-actions:
patterns:
- "*"
The SHA - uses: peter-evans/create-pull-request@c0f553fe549906ede9cf27b5156039d195d2ece0 # v8.1.0Both are pre-existing, neither blocks this PR — happy to defer to a follow-up if you'd rather keep this scoped to the workflow hardening. |
@Kludex happy for you to ignore these btw and merge as they're nits and can be a follow up. will approve now but if you fix now I'll just reapprove :) |
maxisbey
left a comment
maxisbey
left a comment
There was a problem hiding this comment.
approved with above nits as optional fixes, up to you
3 participants
Summary
zizmorworkflow that audits all GitHub Actions workflows on push tomainand on PRs, mirroring the setup in uvicorn.persist-credentials: falseto allactions/checkoutinvocations (artipacked).permissions: contents: readtopublish-pypi.yml(excessive-permissions).github.event.release.*andsteps.*.outputs.resulttemplate expansions incomment-on-release.ymlintoenv:vars consumed viaprocess.env(template-injection).secrets-outside-envon the Claude Code action) is suppressed inline because the proper fix is to putANTHROPIC_API_KEYbehind a dedicated GitHub environment, which is a repo-settings change rather than a code change. Happy to follow up if a maintainer creates the environment.Ran
uvx zizmor .github/workflows/locally; reportsNo findings to report. Good job!after these changes.Test plan
GitHub Actions Security Analysisjob.comment-on-release.ymlstill comments on the right PRs (the script logic is unchanged, just reads values fromprocess.env).AI Disclaimer
This PR was developed with the assistance of either Claude or Codex. I've reviewed and verified the changes.